Release 3.2.0#447
Open
jonasscheid wants to merge 130 commits intomasterfrom
Open
Conversation
Co-authored-by: jonasscheid <43858870+jonasscheid@users.noreply.github.com>
Change Channel. to channel. in all Nextflow scripts
Updated CHANGELOG for version 3.2.0 with changes and dependency updates.
Co-authored-by: jonasscheid <43858870+jonasscheid@users.noreply.github.com>
Update pipeline version in test snapshots from v3.1.0 to v3.2.0dev
Co-authored-by: jonasscheid <43858870+jonasscheid@users.noreply.github.com>
Remove cache buster artifacts from topic channel migration
Merge template 3.5.1
Added function 'strip_modifications' to parse sequences. The --variable_mods flag is no longer required.
Add changelog entry for version 3.2.0dev with a fix.
Added a new version section for 3.2.0dev with details.
Fix #428: add strip_modifications and simplify sequence parsing
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Tuple Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix global FDR null:null precursor length by propagating search params in meta
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…xporter column mismatch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix SUMMARIZE_RESULTS crash with --quantify
…y/ tool-namespaced directories
…lesheet header seed
…put modes Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ch_fasta is a queue channel with one item when --fasta is set. Paired positionally with SUMMARIZE_RESULTS.out.epicore_input (N items), EPICORE runs only once instead of per sample. Convert ch_fasta to a value channel with .first() so it broadcasts. Symptom: with --epicore, only one results/<sample>.tsv file is produced regardless of sample count.
fix: EPICORE runs only once when --fasta is used
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Support PRIDE / SDRF input
Member
|
Warning Newer version of the nf-core template is available. Your pipeline is using an old version of the nf-core template: 3.5.1. For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation. |
|
…ic version lookup. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pin tdf2mzml container to 0.4_noentry
Member
There was a problem hiding this comment.
AI-assisted review
Disclosure: I drafted this review with AI assistance (Claude), then went through each point with the model, cross-checked claims against the code (and in some cases a minimal Nextflow reproducer), and satisfied myself before posting. Severity labels are mine; anything I felt unsure about I either demoted or dropped.
Big, well-scoped release PR — topic-channel migration and search-preset plumbing look good. Inline comments cover the individual findings (two HIGH, five MEDIUM, six LOW). Highlights worth calling out from the summary: the idmassaccuracy meta.yml is copy-pasted from idfilter; the fileconverter version-extraction sed is silently broken (missing backslash escape); resolveSearchParams CLI-override detection has the exact design issues that were debated and rejected in nf-core/differentialabundance#472.
ℹ️ Not inlined — nf-core pipelines lint reports 9 failures, all expected template drift (missing docs/CONTRIBUTING.md, files_unchanged for .gitignore, .prettierignore, linting workflows, logos). Not blocking; nf-core pipelines lint --fix files_unchanged would clear the noise.
A separate follow-up review covers local-module / upstream-contribution concerns.
| def result = new LinkedHashMap(meta) | ||
|
|
||
| searchParamKeys.each { key -> | ||
| def cliOverride = (workflow.commandLine =~ /--${key}[\s=]/).find() |
Member
There was a problem hiding this comment.
🟠 MEDIUM — Regex-matching workflow.commandLine has silent-failure modes worth flagging before release:
-params-file foo.yamlvalues aren't matched — preset silently wins even though the user set the value.- Hyphenated CLI form (
--precursor-mass-tolerance) isn't matched; Nextflow accepts it but the regex keys are underscored. -c overrides.configvalues never reach the regex either.
Same design space was debated in nf-core/differentialabundance#472. @bentsherman's position there was that the Nextflow script sees only resolved params and can't know their source, and that session.cliParams (tried in PR #623) is an internal API that won't survive strict syntax in Nextflow 26.04. Regex parsing was rejected as fragile; schema-default comparison was rejected as silent-failure-prone. Final resolution was a dual-mode split: profiles for single-run, paramsheet for benchmarking.
Options, increasing effort:
- Document the limitation in
docs/usage.md#parameter-precedence. - Drop the override concept — precedence becomes
preset > defaults. - Mirror the differentialabundance split: preset always wins; uniform overrides go via a profile.
Happy to defer, but worth an explicit decision rather than inheriting an approach argued down in a sibling pipeline.
Member
pinin4fjords
left a comment
pinin4fjords
left a comment
There was a problem hiding this comment.
AI-assisted review (Claude, on behalf of @pinin4fjords) — searches against nf-core/modules and OpenMS 3.5.0 source were automated with the model; I verified each claim before posting.
Local modules — upstream contribution
Follow-up to the main review. I automated the relevant nf-core/modules and OpenMS source searches together with the main review; double-checked each claim before posting.
Where a local module is needed, patching the upstream equivalent should usually come before forking it — the bar for an entirely new local module should be higher than for an ext.args / ext.prefix / small upstream PR.
New modules — should be ported upstream before 3.2.0 is tagged
Four of the five new local modules on this PR are generic proteomics utilities:
| New local module | Why upstream |
|---|---|
modules/local/openms/fileconverter |
Pure OpenMS TOPP wrapper. nf-core/openms/ already hosts decoydatabase, filefilter, idfilter, idmerger, idripper, idscoreswitcher, peakpickerhires, peptideindexer, psmfeatureextractor, textexporter. fileconverter is the same shape. |
modules/local/pridepy/download_file |
Generic PRIDE file fetcher. |
modules/local/pridepy/fetch_sdrf |
Generic. |
modules/local/sdrf_pipelines/parse_sdrf |
quantms carries its own local sdrfparsing calling the same parse_sdrf tool with a different subcommand (convert-openms vs convert-mhcquant) and an older pin (0.0.26 vs 0.1.2). A shared upstream module with a subcommand argument would serve both. |
modules/local/openms/idmassaccuracy is the fifth new local module but is excluded: IDMassAccuracy.cpp was removed in OpenMS 3.5.0, the upstream module was deliberately broken with assert false, local port pinned to openms 3.4.1 is correct. (The meta.yml is still wrong — flagged in the main review.)
Existing locals — should be ported upstream before the next release
Not blocking 3.2.0.
Upstream module already exists — switch over, patching upstream where needed
modules/local/openms/textexporter—nf-core/openms/textexporterexists. Output naming difference (${prefix}_exported.tsvvs${prefix}.tsv) isext.prefix.modules/local/openms/psmfeatureextractor—nf-core/openms/psmfeatureextractorexists. Local fork adds a second input (feature_file) withawkto surface MS2Rescore feature names as CLI args. Small upstream PR adding an optional second input.modules/local/untar—nf-core/untarexists. Local fork auto-computes tar depth from a.d/match. Could beext.args2 = '--strip-components=N'or astrip_componentsparam on the upstream module.modules/local/unzip—nf-core/unzipexists. Local emits*.dfrom the working dir instead of${prefix}/.ext.prefix/ output glob.
OpenMS TOPP wrappers not yet upstream — should be contributed
All tools present in OpenMS 3.5.0 source. openmsthirdparty/cometadapter is already upstream as a template.
modules/local/openms/featurefinderidentificationmodules/local/openms/idconflictresolvermodules/local/openms/mapaligneridentificationmodules/local/openms/maprttransformermodules/local/openms/mztabexportermodules/local/openmsthirdparty/percolatoradaptermodules/local/openmsthirdparty/featurelinkerunlabeledkd
Duplicated across proteomics pipelines — one upstream module would serve both
modules/local/tdf2mzml— quantms has its own local copy.modules/local/easypqp/convert,modules/local/easypqp/librarymodules/local/ms2rescore
This was referenced Apr 23, 2026
|
Without having tried the pipeline, the changes look good to me! Do you think the warning "Environment variable SINGULARITYENV_NXF_DEBUG is set, but APPTAINERENV_NXF_DEBUG is preferred" is causing the directory deletion issue that's failing the singularity tests? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AddedFixedEPICORErunning only once instead of per sample when--fastais used, by broadcastingch_fastatoEPICOREvia.first()#446SUMMARIZE_RESULTScrash with--quantifycaused by OpenMS 3.5.0 TextExporter phantom column bug (OpenMS/OpenMS#9120) #444SUMMARIZE_RESULTSdid not work for complex modifications #436ChangedDependencieseasypqpMultiQCNf-coreopenmstdf2mzmlPR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).